Skip to content

Conversation

@m-a-king
Copy link
Collaborator

@m-a-king m-a-king commented Jan 19, 2025

resolved :

📌 과제 설명

JWT의 accessToken 클레임에 roleId를 추가했습니다.
roleIdvolunteerId(pk) 또는 centerId(pk)의 역할을 합니다.
이를 위해서 전체적인 로직 수정과 가독성을 위한 리팩토링을 진행했습니다.

또한 기존의 @currentuser로 처리할 수 있던 Spring Security의 인증 객체의 principal이 변경됨에 따라서 userId와 roleId 각각을 위한 어노테이션을 추가했습니다.

👩‍💻 요구 사항과 구현 내용

액세스 토큰 클레임 추가
어노테이션 추가

✅ PR 포인트 & 궁금한 점

  1. 인증이나 인가 관련 수정은 프레임워크 플로우를 따라가며 확인을 거치는 부분들이 많아서 수정마다 머리가 지끈지끈하네요.
    눈에 띄는 실수가 없는지 가볍게 확인 부탁드리겠습니다!

image image image

어노테이션 사용법

@m-a-king m-a-king self-assigned this Jan 19, 2025
@m-a-king m-a-king linked an issue Jan 19, 2025 that may be closed by this pull request
1 task
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

빌드에 실패했습니다.

@github-actions github-actions bot changed the title [FEATURE] 액세스 토큰 클레임 추가 (역할 아이디) [BUILD FAIL] [FEATURE] 액세스 토큰 클레임 추가 (역할 아이디) Jan 19, 2025
@github-actions github-actions bot closed this Jan 19, 2025
@m-a-king m-a-king reopened this Jan 19, 2025
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

빌드에 실패했습니다.

@github-actions github-actions bot changed the title [BUILD FAIL] [FEATURE] 액세스 토큰 클레임 추가 (역할 아이디) [BUILD FAIL] [BUILD FAIL] [FEATURE] 액세스 토큰 클레임 추가 (역할 아이디) Jan 19, 2025
@github-actions github-actions bot closed this Jan 19, 2025
@m-a-king m-a-king reopened this Jan 20, 2025
@m-a-king m-a-king changed the title [BUILD FAIL] [BUILD FAIL] [FEATURE] 액세스 토큰 클레임 추가 (역할 아이디) [FEATURE] 액세스 토큰 클레임 추가 (역할 아이디) Jan 20, 2025
@m-a-king m-a-king linked an issue Jan 20, 2025 that may be closed by this pull request
2 tasks
Copy link
Collaborator

@ayoung-dev ayoung-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다아


throw new InvalidAuthenticationException(INVALID_PRINCIPAL_TYPE);
}
} No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

개행이 빠졌다고 합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정해씁니다!

throw new InvalidAuthenticationException(AUTHENTICATION_MISSING);
}

if (authentication.getPrincipal() instanceof UserIdentity principal) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

authentication == null를 통과하면
authentication.getPrincipal()도 통과가 가능한건지 여쭤보고 싶습니다
Principal이 비어 있는 경우는 없겠죠?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. authentication == null를 통과하면, authentication.getPrincipal()은 NPE 없이 호출 가능합니다. 또한, principal이 null이거나 비어 있는 값으로 설정될 가능성은 없습니다.

  2. authentication은 Spring Security의 인증 과정에서 생성되며, principal은 생성자에서 초기화됩니다.
    인증이 성공하면 authentication은 항상 principal, credentials, authorities를 포함한 상태로 SecurityContext에 저장됩니다.
    따라서 authentication이 null이 아닌 경우, principal 또한 초기화된 상태입니다.

  3. null instanceof UserIdentity
    authentication.getPrincipal()이 null이라면, instanceof 연산에서 항상 false를 반환하므로, 안전하게 처리됩니다.

  4. Spring Security는 필터 체인에서 인증 객체를 생성한 후 SecurityContext에 저장합니다.
    컨트롤러 레벨에서 접근하는 경우, 이미 인증 객체가 완전히 구성된 상태입니다.
    만약 authentication이 제대로 초기화되지 않았다면, 컨트롤러까지 요청이 도달하지 못합니다.

사실 상 로직에서는 NPE 방어를 진행했고, 인증 객체가 올바르게 구성되지 않는 상황이나 익명 유저(토큰X)는 @Secured로 필터에서 처리되어 예외가 발생할 것 같습니다~


return new InvalidAuthenticationException(INVALID_PRINCIPAL_TYPE);
}
} No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 개행이 빠졌다고 합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정해씁니다!

throw new InvalidAuthenticationException(AUTHENTICATION_MISSING);
}

if (authentication.getPrincipal() instanceof UserIdentity principal) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분도 RoleId 에서 리뷰 남긴 부분이라 코멘트 달아두겠습니다!

@7zrv
Copy link
Collaborator

7zrv commented Jan 20, 2025

머리 아픈 부분들이 많으셨을거 같아요 감사합니다 고생하셨습니다!

Copy link
Collaborator

@leebs0521 leebs0521 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다.

@m-a-king m-a-king force-pushed the feature/306-add-access-token-claims-roleid branch from 6d76494 to feb62fd Compare January 21, 2025 05:38
@sonarqubecloud
Copy link

@m-a-king m-a-king merged commit de4cdeb into main Jan 21, 2025
3 checks passed
@m-a-king m-a-king deleted the feature/306-add-access-token-claims-roleid branch January 21, 2025 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] UserId, RoleId 어노테이션 [FEATURE] (JWT) 액세스 토큰 클레임 추가

5 participants